Skip to content

Conversation

@chikei
Copy link
Contributor

@chikei chikei commented Jul 25, 2025

more like a POC for #5592, I think there are rooms for sharing code/task between this and SonatypeCentralPublishModule

Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chikei Is this code written from scratch? Is it partially copied from other places in the Mill repo? This info would greatly help reviewing it.

@chikei
Copy link
Contributor Author

chikei commented Sep 1, 2025

It's copied from existing SonatypeCentralPublishModule and replace the publishReleases in publishAll using almost identical from publishSnapshot. Effectively changing the publish flow for release from repo1's staging flow to direct http upload.

Edit: and remove the gpg args from publishAll as well

Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the general approach looks good.

I'd like a have the implementations of object MavenPublishModule moved into a trait and just extends that. That way, it should be easier to customize and reuse it in projects.

Also, the hard-coded use of MavenPublishModule.publishAll should instead use a ModuleRef instead.

@maxstreese
Copy link
Contributor

maxstreese commented Oct 23, 2025

Hi @lefou and @chikei , do you still consider this PR actively worked on? If not I would like to attempt this (though it does feel a bit out of my depth).

@lefou regarding your last comment about (1) moving the implementations of object MavenPublishModule into a trait and (2) replace the call to MavenPublishModule.publishAll with a ModuleRef: If I look at the sources of SonatypeCentralPublishModule (which @chikei said MavenPublishModule is in large part a copy of) then both of these points apply to that as well. Therefore I am wondering if adjusting things only here is really desirable? Or should a larger refactoring be done that factors out the commonalities between these two modules and implements both of your remarks?

@lefou
Copy link
Member

lefou commented Oct 23, 2025

Hi @lefou and @chikei , do you still consider this PR actively worked on? If not I would like to attempt this (though it does feel a bit out of my depth).

It would be nice to get this over the finish line. Since the base functionality already done, left work should be mostly mechanical editing.

@lefou regarding your last comment about (1) moving the implementations of object MavenPublishModule into a trait and (2) replace the call to MavenPublishModule.publishAll with a ModuleRef: If I look at the sources of SonatypeCentralPublishModule (which @chikei said MavenPublishModule is in large part a copy of) then both of these points apply to that as well. Therefore I am wondering if adjusting things only here is really desirable? Or should a larger refactoring be done that factors out the commonalities between these two modules and implements both of your remarks?

Sharing common parts between both traits would be the best case, of course. Whether we do it right now or in a subsequent PR is both fine for me, as long as we get it merged rather soon.

lihaoyi pushed a commit that referenced this pull request Nov 19, 2025
)

### Summary

Implements #5592 and builds upon #5601.

### High Level Overview of Changes

This PR implements three traits:

- `MavenPublishModule` is meant to mirror `SonatypeCentralPublishModule`
for plain (!?) Maven repositories
- `MavenModule` contains the actual publishing logic. This has been
taken from parts of `SonatypeCentralPublishModule` and is now extended
by both `MavenPublishModule` as well ass `SonatypeCentralPublishModule`
to share code
- `MavenCredentialsModule` contains tasks for obtaining Maven/Sonatype
credentials. These tasks have been extracted from the
`SonatypeCentralPublishModule` to be shared by both it and the new
`MavenPublishModule`

### Differences between this PR and #5601 

#### Code Structure

#5601 essentially copied parts of `SonatypeCentralPublishModule` to
create `MavenPublishModule`. Apart from duplicating code, this also
created code you may find confusing in places (e.g. by containing a
function named
[publishSnapshot](https://github.com/com-lihaoyi/mill/pull/5601/files#diff-7ffbad225d6fa5d1de00e7ea2066f029c9fe4b90c09785ac3cd51ea0d12b3638R126)
which is used to publish both release and snapshot artifacts). This PR
tries to improve upon that by instead factoring out all code which can
be factored out.

#### Removal of Unused Settings

#5601 defines the tasks `mavenConnectTimeout`, `mavenReadTimeout` and
`mavenAwaitTimeout` as part of the `MavenPublishModule`. These are
however not actually used anywhere. They are used in
`SonatypeCentralPublishModule` and are specific to publishing releases
to Sonatype Central.

#### Bugfix for Choosing the Repo URI

The below was changed after being identified during testing with a
private build.

```diff
- val uri = if (isSnapshot) releaseUri else snapshotUri
+ val uri = if (isSnapshot) snapshotUri else releaseUri
```

### Open Question: Build Compilation Error When Extending
`MavenPublishModule`

When running `./mill dist.installLocalCache` and then using the local
`SNAPSHOT` version of mill including the changes made in this PR in a
build and extending `MavenPublishModule` in a local build module, I
currently get the following exception when running `./mill resolve _` on
that build:

```
[build.mill-59] [error] ## Exception when compiling 23 sources to /home/max/Repositories/github.com/ttmzero/rtp-backend/out/mill-build/compile.dest/classes
[build.mill-59] [error] dotty.tools.dotc.core.MissingType: Cannot resolve reference to type com.lumidion.sonatype.central.client.core.type.SonatypeCredentials.
[build.mill-59] [error] The classfile defining the type might be missing from the classpath.
```

This error can be removed by defining `Deps.sonatypeCentralClient` as a
`mvnDeps` instead of both a `compileMvnDeps` and a `runMvnDeps` in
`libs/scalalib/package.mill` and `libs/javalib/package.mill`. I do not
know why this is the case and would require some help here.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@lihaoyi lihaoyi closed this Nov 19, 2025
lihaoyi pushed a commit that referenced this pull request Nov 22, 2025
…6231)

### Summary

Implements #5592 and builds upon #5601.

### High Level Overview of Changes

This PR implements three traits:

- `MavenPublishModule` is meant to mirror `SonatypeCentralPublishModule`
for generic Maven repositories
- `MavenModule` contains the actual publishing logic. This has been
taken from parts of `SonatypeCentralPublishModule` and is now extended
by both `MavenPublishModule` as well ass `SonatypeCentralPublishModule`
to share code
- `PublishCredentialsModule` contains tasks for obtaining Maven/Sonatype
credentials. These tasks have been extracted from the
`SonatypeCentralPublishModule` to be shared by both it and the new
`MavenPublishModule`

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants